-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exposing stream_format params to user #2811
Exposing stream_format params to user #2811
Conversation
|
||
void VideoProfilesManager::registerVideoSensorProfileFormat(stream_index_pair sip) | ||
{ | ||
if (sip == DEPTH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these formats be hard coded? what will happen if different camera models support different default profiles?
can't we use liberalsense api to fetch/query the default profiles for each sip ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Will do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LibRealSense APIs gives the default profiles of color and depth streams only. Couldn't get those for Infra streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Arun-Prasad-V,
can we query the device about default profiles for the given streams, and if default is not given, then to hard coded it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can query about default profiles and configure the format, if found. If not, maybe can we choose the first found format whose profile's resolution matches with depth stream? So that there won't be anything hard coded.
Arun, I tried testing this PR behavior.
Is this due to this change ? Also, I left a comment about the parameters names. |
found = true; | ||
_formats[sip] = RS2_FORMAT_ANY; | ||
} | ||
else if (string_to_rs2_format(format_str , &temp_format)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this function name to be something more clear.
I expect it only to convert from string to rs2_format, but it return a Boolean that we need to check.
Or, maybe, change the design of these lines..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have updated the design of this function. Will return RS2_FORMAT_ANY if not found. Should I change the function name as well?
Hi @SamerKhshiboun , It occurs not only for the new params, also for old params like "rgb_camera.profile" & "depth_module.profile". |
For example - In D455 model:
|
I understand this, so, in this case, the best thing I can think about is, to define 3 parameters for infra, as default formats |
LGTM |
6a2a0fc
into
IntelRealSense:ros2-development
The user can select the stream_format for all the video streams through below params:
Run
ros2 param describe <your_node_name> <param_name>
command to know the list of formats supported by that particular stream.Alternatively, run
rs-enumerate-devices
command to know the list of profiles supported by the sensors connected.